Use Pulumi to create Entra applications #2248

jemrobinson · 2024-10-22T12:17:47Z

✅ Checklist

You have given your pull request a meaningful title (e.g. Enable foobar integration rather than 515 foobar).
You are targeting the appropriate branch. If you're not certain which one this is, it should be develop.
Your branch is up-to-date with the target branch (it probably was when you started, but it may have changed since then).

🚦 Depends on

n/a

⤴️ Summary

Replace creation of SRE Entra applications through the SDK with creation through pulumi-azuread.

Note we still need one SDK-created Entra application in the SHM which is used to authenticate pulumi-azuread.

The msgraph_permissions map looks up the GUIDs for all possible permissions. We could consider replacing this with a static lookup table that only covers the ones we need.

🌂 Related issues

Closes #2215

🔬 Tests

Tested on a fresh SRE deployment

github-actions · 2024-10-22T12:24:22Z

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
data_safe_haven/commands
pulumi.py
sre.py
data_safe_haven/external/api
credentials.py
graph_api.py
data_safe_haven/infrastructure
project_manager.py
data_safe_haven/infrastructure/components/composite
__init__.py
entra_application.py					23-30, 53, 78, 102-151
data_safe_haven/infrastructure/components/dynamic
__init__.py
data_safe_haven/infrastructure/programs
declarative_sre.py					156
imperative_shm.py
data_safe_haven/infrastructure/programs/sre
entra.py					28-30, 43-47, 57-121
identity.py					39-40
remote_desktop.py					58-59
data_safe_haven/provisioning
sre_provisioning_manager.py
data_safe_haven/types
enums.py
Project Total

_{This report was generated by python-coverage-comment-action}

JimMadge

Looks good I think moving more of this stuff to community supported Pulumi modules will help a lot.

Just a few style questions.

data_safe_haven/infrastructure/components/composite/entra_application.py

data_safe_haven/infrastructure/programs/imperative_shm.py

jemrobinson · 2024-10-28T14:55:23Z

@JimMadge : My biggest worry here is that we add an entry for the Microsoft Graph Service Principal to our state file. This contains a huge amount of data (which more than doubles the size of the state file). Since we're only using it to get the GUIDs for various named permissions, we could replace the bits we need with an Enum or lookup-table. However, this would mean that we would be out-of-sync if any of these GUIDs changed (unlikely to happen as this would break a lot of third-party code).

What do you think?

JimMadge · 2024-10-28T16:04:19Z

@jemrobinson is that the long version of the note about msgraph_permissions in the PR?

How large is it exactly, does it cause problems?
I'd be happy to leave it. Might actually provide some protection against Entra configuration changing.

jemrobinson · 2024-10-28T16:46:23Z

@jemrobinson is that the long version of the note about msgraph_permissions in the PR?

Yes

How large is it exactly, does it cause problems? I'd be happy to leave it. Might actually provide some protection against Entra configuration changing.

About 12k lines (of which we use maybe 4-5 lines) of a 30k JSON file for a full SRE deployment. Also, we should note that the GraphApi class already has a partial copy of this (see below), so abstracting it into a common class could actually simplify our current structure.

data-safe-haven/data_safe_haven/external/api/graph_api.py

Lines 27 to 49 in 1bf9d0c

    
           application_ids: ClassVar[dict[str, str]] = { 
        
               "Microsoft Graph": "00000003-0000-0000-c000-000000000000", 
        
           } 
        
           role_template_ids: ClassVar[dict[str, str]] = { 
        
               "Global Administrator": "62e90394-69f5-4237-9190-012177145e10" 
        
           } 
        
           uuid_application: ClassVar[dict[str, str]] = { 
        
               "Application.ReadWrite.All": "1bfefb4e-e0b5-418b-a88f-73c46d2cc8e9", 
        
               "AppRoleAssignment.ReadWrite.All": "06b708a9-e830-4db3-a914-8e69da51d44f", 
        
               "Directory.Read.All": "7ab1d382-f21e-4acd-a863-ba3e13f7da61", 
        
               "Domain.Read.All": "dbb9058a-0e50-45d7-ae91-66909b5d4664", 
        
               "Group.Read.All": "5b567255-7703-4780-807c-7be8301ae99b", 
        
               "Group.ReadWrite.All": "62a82d76-70ea-41e2-9197-370581804d09", 
        
               "GroupMember.Read.All": "98830695-27a2-44f7-8c18-0c3ebc9698f6", 
        
               "GroupMember.ReadWrite.All": "dbaae8cf-10b5-4b86-a4a1-f871c94c6695", 
        
               "User.Read.All": "df021288-bdef-4463-88db-98f22de89214", 
        
               "User.ReadWrite.All": "741f803b-c850-494e-b5df-cde7c675a1ca", 
        
               "UserAuthenticationMethod.ReadWrite.All": "50483e42-d915-4231-9639-7fdb7fd190e5", 
        
           } 
        
           uuid_delegated: ClassVar[dict[str, str]] = { 
        
               "GroupMember.Read.All": "bc024368-1153-4739-b217-4326f2e966d0", 
        
               "User.Read.All": "a154be20-db9c-4678-8ab7-66f6cc099a59", 
        
           }

…sed consistently

…e used to seed the pulumi-azuread provider

…s since these set the provider information in the state file and cannot be changed

JimMadge · 2024-10-29T10:07:33Z

@jemrobinson That sounds like a good idea then, if the stack doesn't really need the full set 👍.

I'm not super worried though because it is just a few kb in a file we don't expect people to read or edit by hand.

jemrobinson · 2024-10-29T10:37:20Z

@JimMadge : this is actually a bigger refactor than I'd realised. Let's merge this as-is (if you're happy).

jemrobinson requested a review from a team as a code owner October 22, 2024 12:17

jemrobinson changed the title ~~Use Pulumi to create Entra applications~~ WIP Use Pulumi to create Entra applications Oct 22, 2024

jemrobinson marked this pull request as draft October 22, 2024 14:19

jemrobinson force-pushed the 2215-create-entra-applications-with-pulumi branch from b76de51 to 4d7ff34 Compare October 23, 2024 19:30

jemrobinson marked this pull request as ready for review October 23, 2024 21:44

jemrobinson changed the title ~~WIP Use Pulumi to create Entra applications~~ Use Pulumi to create Entra applications Oct 23, 2024

jemrobinson force-pushed the 2215-create-entra-applications-with-pulumi branch from d211fe6 to 810d86e Compare October 25, 2024 09:22

jemrobinson requested review from craddm and JimMadge October 25, 2024 09:28

jemrobinson force-pushed the 2215-create-entra-applications-with-pulumi branch from 59078dd to 3d5f21a Compare October 25, 2024 09:41

JimMadge reviewed Oct 28, 2024

View reviewed changes

jemrobinson added 15 commits October 29, 2024 09:42

🔧 Add GUID for Directory.ReadWrite.All permission

41e3ac4

✨ Add pulumi-azuread EntraApplication component

dc14741

✨ Add support for web applications as well as desktop applications

f533b0e

♻️ Use composite EntraApplication component for identity server

a6ddc98

🚚 Consolidate remote desktop URL construction in Entra component

ba1f5b3

♻️ Use composite EntraApplication component for remote desktop server

5df24d7

⚰️ Drop dynamic EntraApplication component

c43708b

🐛 Use client_id instead of id for Entra applications

c0d9857

♻️ Add an EntraAppPermissionType enum to ensure that Role/Scope are u…

4f9aa97

…sed consistently

🚚 Simplify import path for Entra components

89d9662

⚰️ Drop requirements for GraphAPI everywhere in the SRE where they ar…

463f490

…e used to seed the pulumi-azuread provider

👽 Do not replace azuread:clientId or azuread:tenantId in stack option…

8942257

…s since these set the provider information in the state file and cannot be changed

🔧 Add an EntraSignInAudienceType enum

23918b9

🔊 Add comments explaining application scope requirements

e740ba0

🎨 Add an enum for Entra application IDs

7ef3d6c

jemrobinson force-pushed the 2215-create-entra-applications-with-pulumi branch from bc3db5a to 7ef3d6c Compare October 29, 2024 09:42

♻️ Refactor following suggestions from code review

fd34042

jemrobinson requested a review from JimMadge October 29, 2024 11:14

JimMadge approved these changes Oct 29, 2024

View reviewed changes

jemrobinson merged commit 868d41c into alan-turing-institute:develop Oct 29, 2024
11 checks passed

jemrobinson deleted the 2215-create-entra-applications-with-pulumi branch October 29, 2024 11:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Pulumi to create Entra applications #2248

Use Pulumi to create Entra applications #2248

jemrobinson commented Oct 22, 2024 •

edited

Loading

github-actions bot commented Oct 22, 2024 •

edited

Loading

JimMadge left a comment

jemrobinson commented Oct 28, 2024 •

edited

Loading

JimMadge commented Oct 28, 2024

jemrobinson commented Oct 28, 2024 •

edited

Loading

JimMadge commented Oct 29, 2024

jemrobinson commented Oct 29, 2024

Use Pulumi to create Entra applications #2248

Use Pulumi to create Entra applications #2248

Conversation

jemrobinson commented Oct 22, 2024 • edited Loading

✅ Checklist

🚦 Depends on

⤴️ Summary

🌂 Related issues

🔬 Tests

github-actions bot commented Oct 22, 2024 • edited Loading

Coverage report

JimMadge left a comment

Choose a reason for hiding this comment

jemrobinson commented Oct 28, 2024 • edited Loading

JimMadge commented Oct 28, 2024

jemrobinson commented Oct 28, 2024 • edited Loading

JimMadge commented Oct 29, 2024

jemrobinson commented Oct 29, 2024

jemrobinson commented Oct 22, 2024 •

edited

Loading

github-actions bot commented Oct 22, 2024 •

edited

Loading

jemrobinson commented Oct 28, 2024 •

edited

Loading

jemrobinson commented Oct 28, 2024 •

edited

Loading